Fully generated clients #1
Conversation
|
|
||
| namespace Aws | ||
| { | ||
| namespace Iotjobs |
There was a problem hiding this comment.
weird namespace capitalization
In the model, it's written namespace aws.iotjobs because Java doesn't capitalize namepaces. Neither does python, so that worked well for me. since c++ doesn't really have a style, maybe just stick with all-lowercase?
There was a problem hiding this comment.
we're going to come back to this once we figure out the smithy model namespacing stuff.
| Aws::Crt::Optional<Aws::Crt::String> ThingName; | ||
|
|
||
| private: | ||
| static void LoadFromObject(DescribeJobExecutionRequest &obj, const Crt::JsonView &doc); |
There was a problem hiding this comment.
trivial style questions about static void LoadFromObject:
- Why have this function at all, you could just put the implementation in the constructor?
- Why is this static, but SerializeToObject isn't static?
| class AWS_IOTJOBS_API DescribeJobExecutionRequest final | ||
| { | ||
| public: | ||
| DescribeJobExecutionRequest() = default; |
There was a problem hiding this comment.
super trivial: I heard it helps compile times if you mark things default in the cpp file instead. Otherwise anyone including the header generates the code for the default implementations.
DescribeJobExecutionRequest:: DescribeJobExecutionRequest() = default;
| { | ||
| Aws::Crt::JsonObject jsonObject; | ||
| Execution->SerializeToObject(jsonObject); | ||
| object.WithObject("execution", std::move(jsonObject)); |
There was a problem hiding this comment.
trivial: It might be more graceful if the SerializeToObject() functions just returned a JsonObject, instead of taking one by argument. Then these 3 lines could just be:
object.WithObject("execution", Execution->SerializeToObject())
It would be no less efficent, because return value elision.
| class UpdateJobExecutionResponse; | ||
| class UpdateJobExecutionSubscriptionRequest; | ||
|
|
||
| using OnSubscribeComplete = std::function<void(int ioErr)>; |
There was a problem hiding this comment.
An advantage of using a shared code between services, is that we could have a common Aws::Iot::OnSubscribeComplete type instead of N different Aws::IotserviceX::OnSubscribeComplete types. That would probably make life easier for customers
|
|
||
| bool SubscribeToUpdateJobExecutionAccepted( | ||
| const Aws::Iotjobs::UpdateJobExecutionSubscriptionRequest &request, | ||
| Aws::Crt::Mqtt::QOS qos, |
There was a problem hiding this comment.
I did not let users pick qos, and always used 1. Let's talk offline about how all the SDKs should go
There was a problem hiding this comment.
I don't think we should be in the habbit of making decisions like this for users.... especially C++ users.
Initial work on mqtt bindings for cpp, a sample app, and a simple tes…
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.